Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make prepare wait for add to finish #726

Merged
merged 1 commit into from
Nov 28, 2019

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented Nov 26, 2019

prepare is being run before add command finish because fs.lstat is async and the createProject is resolving before fs.lstat finishes.

This PR changes fs.lstat to fs.lstatSync to make the directory check sync and avoid to resolve before all create work is done.

Closes #725

@l3ender
Copy link

l3ender commented Nov 26, 2019

Nice find! I played around with the prepare/create scripts but couldn't find what was aysnc and breaking the prepare script (not familiar enough with fs, I guess).

@ath0mas
Copy link
Contributor

ath0mas commented Nov 26, 2019

I confirm no more error using your fork & branch, for my previous repro #725 (comment)
💯

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍

Also tested following cases

Environment

$ cordova -v
9.0.0 ([email protected])

Test Cases

Updated config.xml before platform add

$ cordova create iosTest2 com.foobar.iosTest2 iosTest2 && $_

Creating a new cordova project.

$ vi config.xml

--> ADDED <preference name="WKWebViewOnly" value="true" />

$ cordova platform add ../cordova-ios-5.2.0-dev.tgz

Using cordova-fetch for ../cordova-ios-5.2.0-dev.tgz
Warning: using prerelease platform [email protected].
Use 'cordova platform add ios@latest' to add the latest published version instead.
Adding ios project...
Creating Cordova project for the iOS platform:
	Path: platforms/ios
	Package: com.foobar.iosTest2
	Name: iosTest2
iOS project created with [email protected]
Plugin 'cordova-plugin-whitelist' found in config.xml... Migrating it to package.json
Discovered saved plugin "cordova-plugin-whitelist". Adding it to the project
Installing "cordova-plugin-whitelist" for ios
Adding cordova-plugin-whitelist to package.json

$ cordova build ios --release --device

...
** ARCHIVE SUCCEEDED **
...
** EXPORT SUCCEEDED **
...

Updated config.xml after platform add before build

$ cordova create iosTest2 com.foobar.iosTest2 iosTest2 && $_

Creating a new cordova project.

$ cordova platform add ../cordova-ios-5.2.0-dev.tgz

Using cordova-fetch for ../cordova-ios-5.2.0-dev.tgz
Warning: using prerelease platform [email protected].
Use 'cordova platform add ios@latest' to add the latest published version instead.
Adding ios project...
Creating Cordova project for the iOS platform:
	Path: platforms/ios
	Package: com.foobar.iosTest2
	Name: iosTest2
iOS project created with [email protected]
Plugin 'cordova-plugin-whitelist' found in config.xml... Migrating it to package.json
Discovered saved plugin "cordova-plugin-whitelist". Adding it to the project
Installing "cordova-plugin-whitelist" for ios
Adding cordova-plugin-whitelist to package.json

$ vi config.xml

--> ADDED <preference name="WKWebViewOnly" value="true" />

$ cordova build ios --release --device

...
** ARCHIVE SUCCEEDED **
...
** EXPORT SUCCEEDED **
...

@erisu erisu merged commit d68276e into apache:master Nov 28, 2019
@jcesarmobile jcesarmobile deleted the prepare-wait branch November 28, 2019 08:56
erisu pushed a commit that referenced this pull request Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting WKWebViewOnly causes cordova platform add ios to fail
4 participants